refactor: change Material.metadata from string to Uint8Array#178
Merged
refactor: change Material.metadata from string to Uint8Array#178
Conversation
c4e3f33 to
facaa59
Compare
OttoAllmendinger
previously requested changes
Feb 26, 2026
Contributor
OttoAllmendinger
left a comment
There was a problem hiding this comment.
the rust code looks much better
the serde hex utility is more general than just metadata and probably deserves a more general name
| pub fn deserialize<'de, D>(deserializer: D) -> Result<Vec<u8>, D::Error> | ||
| where | ||
| D: Deserializer<'de>, | ||
| { |
Contributor
There was a problem hiding this comment.
this can probably be more general "buffer from hex" or something
Contributor
Author
There was a problem hiding this comment.
Removed the hex_or_bytes deserializer realized we don't actually need it. We're not using hex strings anywhere anymore
e76ef1e to
3912ff0
Compare
metadata is binary data (SCALE-encoded runtime metadata) but was stored as hex strings, requiring constant encoding/decoding. using Uint8Array eliminates this overhead and improves type safety. changes: - TypeScript: Material.metadata is now Uint8Array instead of string - Rust: Material.metadata is now Vec<u8> instead of String - MaterialJs WASM wrapper: accepts &[u8] instead of &str - removed all decode_metadata functions (no longer needed) - test fixtures: added hexToUint8Array() and getWestendMetadata() helpers - no custom serde deserializer needed - standard serde handles byte arrays and Uint8Array benefits: - eliminates hex encode/decode overhead at every use - reduces memory usage (hex strings are 2x the size of raw bytes) - improves type safety (Uint8Array clearly indicates binary data) - matches wasm-utxo conventions (Uint8Array everywhere) - simpler - clients forced to pass bytes, tests use bytes, code accepts bytes BTC-3085 Signed-off-by: Luis Covarrubias <luiscovarrubias@bitgo.com>
3912ff0 to
4c92b2a
Compare
davidkaplanbitgo
approved these changes
Feb 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Metadata is binary data (SCALE-encoded runtime metadata) but was stored as hex-encoded strings, requiring constant encoding/decoding at every boundary. Using Uint8Array eliminates this overhead:
What Changed
TypeScript:
Material.metadata: changed fromstringtoUint8ArrayhexToUint8Array()andgetWestendMetadata()helpersRust:
Material.metadata: changed fromStringtoVec<u8>MaterialJs::new(): accepts&[u8]instead of&strdecode_metadata()functions (no longer needed - bytes passed directly)hex_or_bytesserde deserializer for backwards compatibility (handles hex strings from JSON tests and byte arrays from WASM)Verification
Addresses Otto's PR #145 review feedback: "seriously consider Uint8Array"
Related to: BTC-3064